Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🚸 Support for adding tests to NALAC #629

Merged
merged 24 commits into from
Jun 7, 2024
Merged

🚸 Support for adding tests to NALAC #629

merged 24 commits into from
Jun 7, 2024

Conversation

ystade
Copy link
Collaborator

@ystade ystade commented Jun 7, 2024

Description

This PR contains modifications that were necessary for the tests of the Neutral Atom Logical Array Compiler, see cda-tum/mqt-qmap#470.

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

@ystade ystade mentioned this pull request Jun 7, 2024
Merged
4 tasks
@codecov Codecov
Copy link

codecov bot commented Jun 7, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.6%. Comparing base (7eb0ab7) to head (5ebe377).
Report is 90 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #629   +/-   ##
=====================================
  Coverage   91.6%   91.6%           
=====================================
  Files        148     148           
  Lines      14738   14736    -2     
  Branches    2365    2366    +1     
=====================================
+ Hits       13506   13510    +4     
+ Misses      1232    1226    -6
Flag Coverage Δ
cpp 91.4% <100.0%> (+<0.1%) ⬆️
python 99.7% <ø> (ø)
Files with missing lines Coverage Δ
include/mqt-core/Permutation.hpp 100.0% <100.0%> (ø)
include/mqt-core/na/NAComputation.hpp 100.0% <ø> (ø)
include/mqt-core/na/NADefinitions.hpp 98.0% <ø> (ø)
...clude/mqt-core/na/operations/NAGlobalOperation.hpp 80.0% <ø> (ø)
include/mqt-core/operations/CompoundOperation.hpp 100.0% <ø> (ø)
...nclude/mqt-core/operations/NonUnitaryOperation.hpp 100.0% <ø> (+8.3%) ⬆️
include/mqt-core/operations/Operation.hpp 80.0% <ø> (-1.7%) ⬇️
src/na/NAComputation.cpp 100.0% <100.0%> (ø)
src/na/operations/NALocalOperation.cpp 100.0% <100.0%> (ø)
src/operations/CompoundOperation.cpp 86.4% <100.0%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

@ystade ystade self-assigned this Jun 7, 2024
@ystade ystade added enhancement New feature or request Core Anything related to the Core library and IR c++ Anything related to C++ code labels Jun 7, 2024
@ystade ystade requested a review from burgholzer June 7, 2024 08:45
burgholzer
burgholzer approved these changes Jun 7, 2024
View reviewed changes
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅
Just a couple of comments on covering changes by tests and one slightly bigger comment on whether we can maybe simplify some of the code.
(Started the review before your last commit, so could be that some comments are obsolete by now)

src/na/NAComputation.cpp Show resolved Hide resolved
src/na/operations/NALocalOperation.cpp Show resolved Hide resolved
src/operations/Operation.cpp Outdated Show resolved Hide resolved
src/operations/Operation.cpp Outdated Show resolved Hide resolved
burgholzer
burgholzer requested changes Jun 7, 2024
View reviewed changes
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the overall suggestion and direction. Just some further comments.

include/mqt-core/operations/CompoundOperation.hpp Outdated Show resolved Hide resolved
src/operations/Operation.cpp Outdated Show resolved Hide resolved
@ystade
Copy link
Collaborator Author

ystade commented Jun 7, 2024

And something broke due the change in getUsedQubits :/

burgholzer
burgholzer approved these changes Jun 7, 2024
View reviewed changes
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now ✅

@burgholzer burgholzer merged commit 0e4ff9e into main Jun 7, 2024
34 checks passed
@burgholzer burgholzer deleted the wip-test-na-map branch June 7, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@burgholzer burgholzer burgholzer approved these changes

Assignees

@ystade ystade

Labels
c++ Anything related to C++ code Core Anything related to the Core library and IR enhancement New feature or request
Projects
MQT
Status: Done
MQT Core
Status: Done
Milestone
Neutral Atom Quantum Computing Support
Development

Successfully merging this pull request may close these issues.
2 participants
@ystade @burgholzer